engine/src/function.rs (1)

39-45: Avoid cloning every payload in call_handler.

data is already owned here and isn't reused after dispatch, so data.clone() adds a full extra JSON copy to every invocation on the hot path.

♻️ Proposed fix

     pub async fn call_handler(
         self,
         invocation_id: Option<Uuid>,
         data: Value,
         session: Option<Arc<Session>>,
     ) -> FunctionResult<Option<Value>, ErrorBody> {
-        (self.handler)(invocation_id, data.clone(), session).await
+        (self.handler)(invocation_id, data, session).await
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@engine/src/function.rs` around lines 39 - 45, call_handler is cloning the
owned JSON payload unnecessarily; instead of calling
(self.handler)(invocation_id, data.clone(), session).await, pass the owned data
directly: (self.handler)(invocation_id, data, session).await. Update this call
in the call_handler function (and if the handler signature currently expects a
reference, adjust the handler/type to accept Value by value or otherwise move
the value) so no extra JSON copy is performed on the hot path.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

engine/src/engine/mod.rs (2)

1329-1341: Consider documenting that direct call() invocations have no session context.

Direct EngineTrait::call() passes None for the session parameter. This is appropriate for internal engine operations (triggers, hooks, conditions) that aren't subject to RBAC. However, this means functions invoked directly won't receive session information even if they declare a session parameter.

Consider adding a doc comment to the call method clarifying this behavior:

/// Directly invokes a function by ID.
/// 
/// Note: Direct calls do not propagate session context (session will be `None`).
/// For session-aware invocations, use the worker-based invocation path.
async fn call(...) -> ...

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@engine/src/engine/mod.rs` around lines 1329 - 1341, Add a doc comment to
EngineTrait::call explaining that it purposefully passes None for the session
(via the invocations.handle_invocation call) so direct/internal calls have no
session context; explicitly state “session will be None” and recommend using the
worker-based invocation path for session-aware calls so callers know why session
parameters are not populated.

---

218-224: Consider adding documentation for the session-aware handler feature.

Per coding guidelines for engine/** changes, this new register_function_handler_with_session API and session parameter support should be documented for SDK users. Consider updating:

1. Rust SDK docs explaining how to declare a session parameter in #[function] annotated methods
2. Examples showing session parameter usage: fn my_func(input: MyInput, session: Option<Arc<Session>>)
3. Explanation of when session is available (worker connections with RBAC) vs None (direct calls, workers without session)

Would you like me to draft documentation content for the session parameter feature?

As per coding guidelines: "Check for impacts to the docs/" for engine changes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@engine/src/engine/mod.rs` around lines 218 - 224, Add user-facing
documentation for the new session-aware API: document the
register_function_handler_with_session function and the session parameter
semantics in the Rust SDK docs, include a small example showing a handler
signature like fn my_func(input: MyInput, session: Option<Arc<Session>>) and
note how to annotate with #[function], and describe when session is Some (worker
connections with RBAC-enabled sessions) versus None (direct calls or workers
without session). Update any relevant examples and the engine-level API docs to
reference Session, SessionHandler, and SessionHandlerFn so SDK users know how to
declare and consume the session parameter.

engine/function-macros/src/lib.rs (1)

378-416: Fully qualify the Session type in generated code for better self-containment.

The generated closure (line 381) uses ::std::sync::Arc<Session> where Session is unqualified. While this works because files using the macro with session parameters explicitly import Session (e.g., use crate::modules::worker::rbac_session::Session), the generated code relies on this implicit import requirement. Fully qualifying Session would eliminate this hidden dependency and provide clearer, more robust code.

♻️ Suggested fix

-                            let `#handler_ident` = SessionHandler::new(move |input: Value, session: Option<::std::sync::Arc<Session>>| {
+                            let `#handler_ident` = SessionHandler::new(move |input: Value, session: Option<::std::sync::Arc<crate::modules::worker::rbac_session::Session>>| {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@engine/function-macros/src/lib.rs` around lines 378 - 416, The generated
closure uses an unqualified Session type; change the closure signature and all
references of ::std::sync::Arc<Session> inside handler_and_registration (the
SessionHandler::new invocation and any return/param types used there) to the
fully-qualified path (e.g., ::crate::modules::worker::rbac_session::Session) so
the macro output is self-contained; update both the closure parameter type and
any Arc<...> occurrences before calling
engine.register_function_handler_with_session to use the fully-qualified Session
type.